feat: add pre-steps and post-steps to threat detection job#24250
feat: add pre-steps and post-steps to threat detection job#24250
Conversation
|
Hey That said, this is still in its earliest planning stage (single "Initial plan" commit, no file changes, 0 lines of code). Here are the things that will need to land before this is ready for review:
If you'd like a hand, here's a ready-to-use prompt you can use to drive the implementation:
|
- Add `PostSteps` field to `ThreatDetectionConfig` for post-execution steps - Change `Steps` field semantics: now runs BEFORE engine execution (pre-steps) - Update `HasRunnableDetection` to check both `Steps` and `PostSteps` - Update `parseThreatDetectionConfig` to parse `post-steps` frontmatter field - Update `buildDetectionJobSteps` to place Steps before and PostSteps after engine execution - Update JSON schema to add `post-steps` field and update `steps` description - Update documentation in threat-detection.md and frontmatter-full.md - Update existing test and add new tests for pre/post step ordering Agent-Logs-Url: https://github.com/github/gh-aw/sessions/afee0692-364c-4f5e-b0d1-7add5d35cdca Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds support for both pre-execution and post-execution custom steps in the threat-detection job, aligning its behavior with the agent job and enabling workflow setup/teardown around the detection engine run.
Changes:
- Updates threat-detection config to treat
stepsas pre-steps and introducespost-stepsfor post-execution steps. - Adjusts detection job step generation and runnable detection checks to account for both pre/post steps.
- Updates JSON schema and documentation to describe the new fields and execution order; expands tests for parsing and ordering.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/threat_detection.go | Adds PostSteps, updates parsing and detection job step ordering to run pre-steps before engine and post-steps after. |
| pkg/workflow/threat_detection_test.go | Extends parsing tests for post-steps and updates ordering tests for pre vs post placement. |
| pkg/parser/schemas/main_workflow_schema.json | Adds post-steps to threat-detection schema and updates steps description to pre-execution semantics. |
| docs/src/content/docs/reference/threat-detection.md | Documents new post-steps field, updates examples and execution order. |
| docs/src/content/docs/reference/frontmatter-full.md | Updates reference frontmatter to include post-steps and updated steps meaning. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Step 4: Custom pre-steps if configured (run before engine execution) | ||
| if len(data.SafeOutputs.ThreatDetection.Steps) > 0 { | ||
| steps = append(steps, c.buildCustomThreatDetectionSteps(data.SafeOutputs.ThreatDetection.Steps)...) | ||
| } |
There was a problem hiding this comment.
Custom threat-detection steps generated via buildCustomThreatDetectionSteps() are inserted without the detectionStepCondition if: guard. As a result, user-provided pre-steps will run even when the detection_guard determines run_detection == 'false' (e.g., when there are no agent outputs/patches), which can cause unexpected side effects or failures in runs where threat detection should be skipped. Consider injecting if: always() && steps.detection_guard.outputs.run_detection == 'true' into these custom steps by default (unless the user already provided an explicit if in the step map).
| t.Run("post-steps come after engine execution and before upload", func(t *testing.T) { | ||
| data := &WorkflowData{ | ||
| SafeOutputs: &SafeOutputsConfig{ | ||
| ThreatDetection: &ThreatDetectionConfig{ | ||
| PostSteps: []any{ | ||
| map[string]any{ | ||
| "name": "Custom Post Scan", | ||
| "run": "echo 'Custom post-scanning...'", | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| steps := compiler.buildDetectionJobSteps(data) | ||
|
|
||
| if len(steps) == 0 { | ||
| t.Fatal("Expected non-empty steps") | ||
| } | ||
|
|
||
| stepsString := strings.Join(steps, "") | ||
|
|
||
| postStepPos := strings.Index(stepsString, "Custom Post Scan") | ||
| setupStepPos := strings.Index(stepsString, "Setup threat detection") | ||
| uploadStepPos := strings.Index(stepsString, "Upload threat detection log") | ||
| concludeStepPos := strings.Index(stepsString, "Parse and conclude threat detection") | ||
|
|
||
| if postStepPos == -1 { | ||
| t.Error("Expected to find 'Custom Post Scan' step") | ||
| } | ||
| if setupStepPos == -1 { | ||
| t.Error("Expected to find 'Setup threat detection' step") | ||
| } | ||
| if uploadStepPos == -1 { | ||
| t.Error("Expected to find 'Upload threat detection log' step") | ||
| } | ||
| if concludeStepPos == -1 { | ||
| t.Error("Expected to find 'Parse and conclude threat detection' step") | ||
| } | ||
|
|
||
| // Verify ordering: post-steps should come after setup and before upload/conclude | ||
| if postStepPos < setupStepPos { | ||
| t.Errorf("Custom post-steps should come after 'Setup threat detection'. Got post-step at position %d, setup at position %d", postStepPos, setupStepPos) | ||
| } | ||
| if postStepPos > uploadStepPos { |
There was a problem hiding this comment.
This test case name claims post-steps come after engine execution, but the assertions only check ordering relative to the "Setup threat detection" step. That would still pass if post-steps were accidentally moved to run before the engine execution step. Consider asserting ordering relative to a stable marker from the engine step output (e.g., the injected step id "detection_agentic_execution" or another deterministic string from buildDetectionEngineExecutionStep).
|
@copilot review comments |
…est ordering assertions - Inject detectionStepCondition into custom threat detection steps (pre/post) unless the user already provides an if: condition. This ensures custom steps are skipped when detection_guard says run_detection is false. - Update test to verify post-step ordering relative to id: detection_agentic_execution (stable engine step marker) instead of just "Setup threat detection" - Add TestCustomThreatDetectionStepsGuardCondition to explicitly test condition injection and preservation Agent-Logs-Url: https://github.com/github/gh-aw/sessions/db33799b-37fd-4489-96df-09ea24078ed9 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Addressed both review comments in commit 92d06e0:
|
The
detectionjob only supportedstepsrunning after AI analysis, unlike theagentjob which supports bothsteps(pre-execution) andpost-steps(post-execution). This made it impossible to do setup work (e.g., connecting to a private AI gateway) before the detection engine runs.Changes
stepssemantics changed: now runs before engine execution (pre-steps), matchingagentjob behaviorpost-stepsfield: runs after engine execution, wherestepspreviously ranHasRunnableDetection: updated to check bothStepsandPostStepspost-steps, updatedstepsdescriptionthreat-detection.mdandfrontmatter-full.mdwith new fields, examples, and execution orderNew execution order
Example
Important
This is a breaking change for existing workflows using
stepsunderthreat-detection: those steps previously ran after AI analysis and will now run before it. Users relying on post-execution behavior should migrate topost-steps.Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
https://api.github.com/graphql/usr/bin/gh /usr/bin/gh api graphql -f query=query($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { hasDiscussionsEnabled } } -f owner=github -f name=gh-aw(http block)/usr/bin/gh /usr/bin/gh api graphql -f query=query($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { hasDiscussionsEnabled } } -f owner=github -f name=gh-aw 684540fb(http block)/usr/bin/gh /usr/bin/gh api graphql -f query=query($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { hasDiscussionsEnabled } } -f owner=github -f name=gh-aw er(http block)https://api.github.com/orgs/test-owner/actions/secrets/usr/bin/gh gh api /orgs/test-owner/actions/secrets --jq .secrets[].name '**/*.ts' '**/*.json' --ignore-path ../../../.pr**/*.json 1909794/b114/vet.cfg ache/go/1.25.0/x64/pkg/tool/linux_amd64/vet Action pins syngit(http block)https://api.github.com/repos/actions/ai-inference/git/ref/tags/v1/usr/bin/gh gh api /repos/actions/ai-inference/git/ref/tags/v1 --jq .object.sha --show-toplevel /opt/hostedtoolcache/go/1.25.0/xrev-parse /usr/bin/git ub/workflows -buildtags me: String!) { --show-toplevel git init�� -nilfunc bash /opt/hostedtoolcache/node/24.14.0/x64/bin/node ithub/workflows on repository(owne--show-toplevel /opt/hostedtoolcache/node/24.14.0/x64/bin/node(http block)https://api.github.com/repos/actions/checkout/git/ref/tags/v3/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v3 --jq .object.sha -test.paniconexit0 -test.v=true om/testowner/testrepo.git -test.timeout=10git -test.run=^Test -test.short=true--show-toplevel /opt/hostedtoolcache/go/1.25.0/x64/pkg/tool/linux_amd64/compile -o /tmp/go-build337676493/b430/_pkg_.a -trimpath /usr/bin/git -p github.com/githurev-parse -lang=go1.25 git(http block)https://api.github.com/repos/actions/checkout/git/ref/tags/v5/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v5 --jq .object.sha ithub/workflows --write 64/pkg/tool/linux_amd64/vet !../../../pkg/woinfocmp --ignore-path ../../../.prettixterm-color 64/pkg/tool/linux_amd64/vet -c k/gh-aw/gh-aw/.github/workflows log 64/pkg/tool/linux_amd64/compile -n1 --format=format:rev-parse 5ebc5b2e7c2b 64/pkg/tool/linux_amd64/compile(http block)/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v5 --jq .object.sha download 1 /usr/bin/git test-logs/run-1 -tests /usr/lib/sysstat--show-toplevel git rev-�� --show-toplevel debian-sa1 /usr/bin/git ithub/workflows(http block)/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v5 --jq .object.sha --show-toplevel /usr/bin/gh /usr/bin/git graphql -f e/git git rev-�� --show-toplevel e/git /usr/bin/git view tname) 64/pkg/tool/linu--show-toplevel git(http block)https://api.github.com/repos/actions/checkout/git/ref/tags/v6/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v6 --jq .object.sha --show-toplevel /opt/hostedtoolc-tests /usr/bin/git -unreachable=falgit /tmp/go-build288rev-parse ache/node/24.14.--show-toplevel git rev-�� --show-toplevel /opt/hostedtoolcache/go/1.25.0/xconfig /usr/bin/git ithub/workflows /tmp/go-build288rev-parse k/gh-aw/node_mod--show-toplevel git(http block)/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v6 --jq .object.sha /tmp/go-build337676493/b456/_pkg_.a -trimpath /usr/bin/git -p github.com/githurev-parse -lang=go1.25 git rev-�� --show-toplevel -goversion /usr/bin/git -c=4 -nolocalimports -importcfg git(http block)/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v6 --jq .object.sha --show-toplevel git /usr/bin/git /home/REDACTED/worgit rev-parse /usr/bin/infocmp--show-toplevel git rev-�� --show-toplevel infocmp /usr/bin/git xterm-color x_amd64/vet e/git git(http block)https://api.github.com/repos/actions/github-script/git/ref/tags/v8/usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --jq .object.sha se 1909794/b030/vet.cfg n-dir/node(http block)/usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --jq .object.sha se 1909794/b188/vetowner=github ache/go/1.25.0/x-f(http block)/usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --jq .object.sha se 1909794/b231/vet.cfg ache/go/1.25.0/x64/pkg/tool/linux_amd64/vet(http block)https://api.github.com/repos/actions/setup-go/git/ref/tags/v4/usr/bin/gh gh api /repos/actions/setup-go/git/ref/tags/v4 --jq .object.sha /repos/actions/github-script/git/ref/tags/v8 --jq /usr/bin/git 345 /tmp/go-build288rev-parse k/gh-aw/gh-aw/no--show-toplevel git rev-�� --show-toplevel /opt/hostedtoolcrev-parse /usr/bin/git ithub/workflows /tmp/go-build288rev-parse in/sh git(http block)https://api.github.com/repos/actions/setup-node/git/ref/tags/v4/usr/bin/gh gh api /repos/actions/setup-node/git/ref/tags/v4 --jq .object.sha /tmp/shared-actions-test3078004568 rev-parse /usr/bin/git -unreachable=falgit /tmp/go-build288rev-parse k/gh-aw/gh-aw/ac--show-toplevel git rev-�� --show-toplevel /opt/hostedtoolcconfig /usr/bin/git -unreachable=falgit /tmp/go-build288rev-parse repository(owne--show-toplevel git(http block)https://api.github.com/repos/astral-sh/setup-uv/git/ref/tags/eac588ad8def6316056a12d4907a9d4d84ff7a3b/usr/bin/gh gh api /repos/astral-sh/setup-uv/git/ref/tags/eac588ad8def6316056a12d4907a9d4d84ff7a3b --jq .object.sha h ../../../.pret.prettierignore assword=$GITHUB_--log-level=error de http.https://gitbash(http block)https://api.github.com/repos/github/gh-aw/usr/bin/gh gh api /repos/github/gh-aw --jq .visibility on' --ignore-path ../../../.prettierignore .cfg r: $owner, name: $name) { hasDiscussionsEnabled } }(http block)https://api.github.com/repos/github/gh-aw-actions/git/ref/tags/v0/usr/bin/gh gh api /repos/github/gh-aw-actions/git/ref/tags/v0 --jq .object.sha --get-regexp --local $name) { hasDiscussionsEnabled } }(http block)https://api.github.com/repos/github/gh-aw-actions/git/ref/tags/v0.1.2/usr/bin/gh gh api /repos/github/gh-aw-actions/git/ref/tags/v0.1.2 --jq .object.sha k/gh-aw/gh-aw/.github/workflows/blog-auditor.md /opt/hostedtoolc-goversion /usr/bin/git -unreachable=falgit /tmp/go-build288rev-parse k/gh-aw/gh-aw/ac--show-toplevel git init��(http block)https://api.github.com/repos/github/gh-aw-actions/git/ref/tags/v1.0.0/usr/bin/gh gh api /repos/github/gh-aw-actions/git/ref/tags/v1.0.0 --jq .object.sha HLfNLuUO- -f /opt/hostedtoolcache/go/1.25.0/x64/pkg/tool/linux_amd64/vet -f owner=github -f rtcfg -V=f�� g/timeutil/format.go g/timeutil/format_test.go ache/node/24.14.0/x64/bin/node stall-gh-aw.sh tgit(http block)https://api.github.com/repos/github/gh-aw-actions/git/ref/tags/v1.2.3/usr/bin/gh gh api /repos/github/gh-aw-actions/git/ref/tags/v1.2.3 --jq .object.sha licyBlockedUsersApprovalLabelsCompiledOutput3367849554/001 rev-parse cfg --local .cfg $name) { has--show-toplevel /opt/hostedtoolcache/go/1.25.0/x64/pkg/tool/linux_amd64/compile -o -aw/git/ref/tags/v3.0.0 -trimpath 0/x64/bin/node -p github.com/githurev-parse -lang=go1.25 git-upload-pack(http block)https://api.github.com/repos/github/gh-aw/actions/runs/1/artifacts/usr/bin/gh gh run download 1 --dir test-logs/run-1 -tests /usr/lib/sysstat-buildmode=exe(http block)https://api.github.com/repos/github/gh-aw/actions/runs/12345/artifacts/usr/bin/gh gh run download 12345 --dir test-logs/run-12345 -tests /home/REDACTED/work/gh-aw/gh-aw/node_modules/.bin/node(http block)https://api.github.com/repos/github/gh-aw/actions/runs/12346/artifacts/usr/bin/gh gh run download 12346 --dir test-logs/run-12346(http block)https://api.github.com/repos/github/gh-aw/actions/runs/2/artifacts/usr/bin/gh gh run download 2 --dir test-logs/run-2 l /home/REDACTED/go/bin/node(http block)https://api.github.com/repos/github/gh-aw/actions/runs/3/artifacts/usr/bin/gh gh run download 3 --dir test-logs/run-3 -tests /opt/hostedtoolcache/node/24.14.0/x64/bin/node(http block)https://api.github.com/repos/github/gh-aw/actions/runs/4/artifacts/usr/bin/gh gh run download 4 --dir test-logs/run-4 -tests(http block)https://api.github.com/repos/github/gh-aw/actions/runs/5/artifacts/usr/bin/gh gh run download 5 --dir test-logs/run-5 -tests /bin/sh l(http block)https://api.github.com/repos/github/gh-aw/actions/workflows/usr/bin/gh gh workflow list --json name,state,path '**/*.ts' '**/*.json' --ignore-path ../../../.pr**/*.json 1909794/b091/vet.cfg ache/go/1.25.0/x64/pkg/tool/linux_amd64/vet(http block)/usr/bin/gh gh run list --json databaseId,number,url,status,conclusion,workflowName,createdAt,startedAt,updatedAt,event,headBranch,headSha,displayTitle --workflow nonexistent-workflow-12345 --limit 100(http block)/usr/bin/gh gh run list --json databaseId,number,url,status,conclusion,workflowName,createdAt,startedAt,updatedAt,event,headBranch,headSha,displayTitle --workflow nonexistent-workflow-12345 --limit 6(http block)https://api.github.com/repos/github/gh-aw/git/ref/tags/v0.47.4/usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v0.47.4 --jq .object.sha --show-toplevel sh /usr/bin/git ithub/workflows(http block)https://api.github.com/repos/github/gh-aw/git/ref/tags/v1.0.0/usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v1.0.0 --jq .object.sha ithub/workflows(http block)https://api.github.com/repos/github/gh-aw/git/ref/tags/v1.2.3/usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v1.2.3 --jq .object.sha --noprofile(http block)https://api.github.com/repos/github/gh-aw/git/ref/tags/v2.0.0/usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v2.0.0 --jq .object.sha(http block)/usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v2.0.0 --jq .object.sha echo "Syncing install-gh-aw.sh to actions/setup-cli/install.sh..."(http block)/usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v2.0.0 --jq .object.sha *.json' '!../../-errorsas log k/_temp/uv-pytho-nilfunc --format=%H:%ct(http block)https://api.github.com/repos/github/gh-aw/git/ref/tags/v3.0.0/usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v3.0.0 --jq .object.sha if [ -f .github/-p(http block)https://api.github.com/repos/githubnext/agentics/git/ref/tags//usr/bin/gh gh api /repos/githubnext/agentics/git/ref/tags/# --jq .object.sha w/js/**/*.json' --ignore-path --local $name) { hasDiscussionsEnabled } } user.email(http block)https://api.github.com/repos/nonexistent/action/git/ref/tags/v999.999.999/usr/bin/gh gh api /repos/nonexistent/action/git/ref/tags/v999.999.999 --jq .object.sha ty-test.md(http block)https://api.github.com/repos/nonexistent/repo/actions/runs/12345/usr/bin/gh gh run view 12345 --repo nonexistent/repo --json status,conclusion(http block)https://api.github.com/repos/owner/repo/actions/workflows/usr/bin/gh gh workflow list --json name,state,path --repo owner/repo 8d519d9/node_modules/.bin/pretti-f(http block)/usr/bin/gh gh workflow list --json name,state,path --repo owner/repo de_modules/.bin/node(http block)https://api.github.com/repos/owner/repo/contents/file.md/tmp/go-build337676493/b396/cli.test /tmp/go-build337676493/b396/cli.test -test.testlogfile=/tmp/go-build337676493/b396/testlog.txt -test.paniconexit0 -test.v=true -test.parallel=4 -test.timeout=10m0s -test.run=^Test -test.short=true(http block)https://api.github.com/repos/test-owner/test-repo/actions/secrets/usr/bin/gh gh api /repos/test-owner/test-repo/actions/secrets --jq .secrets[].name '**/*.ts' '**/*.json' --ignore-premote.origin.url --others 86_64/sh(http block)If you need me to access, download, or install something from one of these locations, you can either: